-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[Website 2.0] General Version Dropdown #17948
[Website 2.0] General Version Dropdown #17948
Conversation
Hey @connorgoggins , Thanks for submitting the PR
CI supported jobs: [clang, edge, centos-gpu, centos-cpu, windows-cpu, miscellaneous, unix-cpu, windows-gpu, unix-gpu, website, sanity] Note: |
Thanks for introducing the versioned website. In terms of appearance, could we make the version selector more visually distinct from other links? From the current look it doesn't seem obvious that a dropdown menu is there. Another ask is that since we are maintaining both the mxnet 1.x (v1.x branch, w/ 1.7 to be the next version) and 2.x (master branch, w/ 2.0 to be the next version), can we make nightly builds for these dev branches available? See #17701 for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some organizational preferences for styles... so it is easier to maintain. Can you do that in a followup PR?
@@ -7,6 +7,27 @@ | |||
<meta charset="utf-8"> | |||
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no"> | |||
<meta http-equiv="x-ua-compatible" content="ie=edge"> | |||
<style> | |||
.dropdown { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make sure all of the style stuff is documented - so we know what it does and how it interacts with all of the other pieces? Like how does this interact with this?
https://github.com/apache/incubator-mxnet/blob/master/docs/python_docs/_static/mxnet.css
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll create a document/wiki page with styling info once the design is finalized.
@@ -15,4 +15,25 @@ | |||
|
|||
<script src="{{'/assets/js/clipboard.js'|relative_url}}"></script> | |||
<script src="{{'/assets/js/copycode.js'|relative_url}}"></script> | |||
<style> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated in layout.html. Should be abstracted to a single source so it's easier to manage.
<div class="dropdown"> | ||
<span>master</span> | ||
<div class="dropdown-content"> | ||
<a style="color:#FF4500;" href="/">master</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to central css
<div class="dropdown"> | ||
<span style="color:#FFFFFF">master</span> | ||
<div class="dropdown-content"> | ||
<a style="color:#FF4500;font-weight: lighter;" href="/">master</a><br> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move styles to central css file?
@szha thanks for your feedback! I would be happy to modify the styling of the version dropdown. Can you give me an idea of what exactly you had in mind? Regarding the nightly build configurations, since v1.x and v2.x use restricted nodes (see Jenkinsfile_website_full) these builds would need to be run on prod Jenkins, which I currently do not have admin access to. At the moment, I am trying to get access to prod Jenkins so I can set up these nightly build pipelines. |
@szha I created two PRs with changes that will add Jenkinsfiles to support nightly builds for v1.x and v2.x (master). When you have some free time, would you mind reviewing them? |
@@ -20,7 +20,7 @@ all: html | |||
html: | |||
mkdir -p build | |||
cd src && bundle install && JEKYLL_ENV=production bundle exec jekyll build --config _config_prod.yml -d ../build/html && cd .. | |||
|
|||
wget https://mxnet-static-artifacts.s3.amazonaws.com/versions.zip && unzip versions.zip -d build/html && rm -rf build/html/__MACOSX && rm versions.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does that come from? It seems like we're breaking out of a context here. S3 should not be the method how multiple Jenkins jobs exchange data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your question, @marcoabreu. The content for the static artifacts is managed in this repository. Originally this line was a call to directly clone the repository, but based on feedback it was decided that it would be better to not rely on external repositories and pull the artifacts from an S3 bucket instead.
Here, S3 isn't being used as the way multiple Jenkins jobs exchange data per se. There are many older static artifacts in the ZIP that have gone through multiple rounds of changes which are tracked in the aforementioned repository, rather than simply being built by another Jenkins pipeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's an idea: add this to the Dockerfile so that it gets cached as part of the image and we don't have to grab that zip every time there's a test or build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But how would you invalidate that cache?
I think it's important to have continuity to allow a contributor to understand and also reproduce how our website gets created. As it stands right now, people would not be able to facilitate this.
What happened to the idea with the Jenkins jobs creating artifacts and another job consuming these artifacts to then assemble the website?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this is a static artifact that can't be directly updated by the community. For a complete solution, we definitely need them to be generated.
On the other hand, the lack of website versioning is such a pressing issue that it has come up multiple times, even in past release votes, and is still actively causing confusion for many current and prospective users. We must be losing user base because of this.
If the contributors could come up with a plan for a solution that's based on automatically generated content, and execute the plan before any of our next releases, I think we should accept this solution for now to address the immediate need for avoiding confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that @connorgoggins is already contributing other PRs in this direction (#17956 #17957), I trust that the contributor has the intention to make the generation automated. @connorgoggins @sandeep-krishnamurthy contingent on if you could spell out a plan for automated generation for the website, I support accepting this patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed @szha. The priority right now should be getting the working version dropdown in this PR into the production site as soon as possible.
@marcoabreu, I have created a confluence page so that all contributors can understand and reproduce the steps to add additional releases to the website. In the future, we will move towards creating dynamic pipelines to automatically generate and integrate new releases of MXNet into the website, but for now I believe the static artifact solution + master integration in this PR serves its purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcoabreu Thanks for your feedback and I fully agree that if generation of web content happens through Jenkins in an automated way, all community members can participate in the updates. In that direction, @connorgoggins already has PRs to make 1.x and 2.x branches auto builds. The fix here is very critical for MXNet community. Without this change, users are unable to use MXNet website. One concern would be if there is a patch release on 1.6.x then someone needs to regenerate static artifacts. Given that @connorgoggins already documented steps for the same and his other contributions I see this as a non-blocking issue that will not leave MXNet community in limbo.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. Thanks
Update: the new version of the website preview reflects the changes I have made to this PR to incorporate feedback. @szha the styling of the dropdown in 1.6 and master has been updated to more clearly indicate that the menu option is a dropdown. |
To me it's unclear how that magic versions.zip gets created. Could you maybe make a diagram of how the individual versions get created, where they are stored and how they are merged? Some data flow would be good |
@marcoabreu see "Jenkins Artifact Generation" within the Static Artifact Publication section of the wiki page for detailed instructions on how to add a new artifact, create |
I'm no longer an Amazon employee. This content should be hosted on confluence. |
The updated style looks good to me. Thanks @connorgoggins |
@marcoabreu the Confluence page with instructions for adding releases is now public. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contributions!
e046e7c
to
398beb5
Compare
398beb5
to
57a7a53
Compare
Description
This PR resolves the issues outlined in the Github issue for the MXNet Website 2.0 General Version Dropdown. It serves to add a fully functional API version dropdown with consistent styling to the master website, and also modifies the Jenkins build instructions to download static artifacts from an external repository. Currently, this repository of static artifacts includes websites for the following past versions of the MXNet API:
Each version of the website also has a similarly formatted API version dropdown in the header of each page, making it easy for users to seamlessly navigate between different versions of the MXNet website.
Checklist
Essentials
Changes
Comments
Preview is currently available here.
For instructions on adding future release versions (either as static artifacts or via dynamic build pipelines) please read the wiki page or the Confluence page.
@sandeep-krishnamurthy @aaronmarkham @szha @sojiadeshina @leezu